Skip to content

Conversation

@jack-berg
Copy link
Member

Followup to #5566.

@jack-berg jack-berg requested a review from a team as a code owner November 14, 2024 18:50
@opentelemetrybot opentelemetrybot requested review from a team November 14, 2024 18:51
Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but I am just double checking that updating the opentelemetry-java-examples submodule is on purpose?

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jack-berg - if I execute npm run code-excerpts, then I get the following page updates:

diff --git a/content/en/docs/languages/go/getting-started.md b/content/en/docs/languages/go/getting-started.md
index 0294d82c..b122280a 100644
--- a/content/en/docs/languages/go/getting-started.md
+++ b/content/en/docs/languages/go/getting-started.md
@@ -377,25 +377,22 @@ Modify `rolldice.go` to include custom instrumentation using OpenTelemetry API:
 package main
 
 import (
-       "fmt"
-       "io"
-       "log"
-       "math/rand"
-       "net/http"
-       "strconv"
-
        "go.opentelemetry.io/contrib/bridges/otelslog"
        "go.opentelemetry.io/otel"
-       "go.opentelemetry.io/otel/attribute"
        "go.opentelemetry.io/otel/metric"
+       "go.opentelemetry.io/otel/sdk/instrumentation"
 )
 
-const name = "go.opentelemetry.io/otel/example/dice"
+const name = "rolldice"
 
 var (
        tracer = otel.Tracer(name)
        meter  = otel.Meter(name)
-       logger = otelslog.NewLogger(name)
+       logger = otelslog.NewLogger(
+               otelslog.WithInstrumentationScope(instrumentation.Scope{
+                       Name: name,
+               }),
+       )
        rollCnt metric.Int64Counter
 )
 
@@ -408,30 +405,6 @@ func init() {
                panic(err)
        }
 }
-
-func rolldice(w http.ResponseWriter, r *http.Request) {
-       ctx, span := tracer.Start(r.Context(), "roll")
-       defer span.End()
-
-       roll := 1 + rand.Intn(6)
-
-       var msg string
-       if player := r.PathValue("player"); player != "" {
-               msg = fmt.Sprintf("%s is rolling the dice", player)
-       } else {
-               msg = "Anonymous player is rolling the dice"
-       }
-       logger.InfoContext(ctx, msg, "result", roll)
-
-       rollValueAttr := attribute.Int("roll.value", roll)
-       span.SetAttributes(rollValueAttr)
-       rollCnt.Add(ctx, 1, metric.WithAttributes(rollValueAttr))
-
-       resp := strconv.Itoa(roll) + "\n"
-       if _, err := io.WriteString(w, resp); err != nil {
-               log.Printf("Write failed: %v\n", err)
-       }
-}
 ```
 <!-- prettier-ignore-end -->

Is this expected?

@jack-berg
Copy link
Member Author

@chalin we've run into this question before here: #5276 (comment)

Every time I run npm run code-excerpts I have to manually revert this snippet in ./go/getting-started.md. At this point, it seems like we might consider:

  • Removing the go excerpt and instead just keeping it as a hard-coded code snippet
  • Reverting the go content module to a version aligned with the current excerpt.

@chalin
Copy link
Contributor

chalin commented Nov 18, 2024

Sigh, right. Let me create an issue for that.

@chalin
Copy link
Contributor

chalin commented Nov 18, 2024

The issue is:

And the PR to implement your first proposition @jack-berg is:

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the Go code excerpt issue (#5629) is orthogonal to this PR's purpose, and it being treated separately, this PR is a go for me!

@chalin chalin requested a review from svrnm November 18, 2024 23:09
@chalin chalin force-pushed the java-cardinality-limits branch from 2b66879 to 0fbce49 Compare November 18, 2024 23:09
@opentelemetrybot opentelemetrybot requested a review from a team November 18, 2024 23:10
@theletterf theletterf added this pull request to the merge queue Nov 19, 2024
Merged via the queue into open-telemetry:main with commit 03e7a5b Nov 19, 2024
17 checks passed
drewby pushed a commit to drewby/opentelemetry.io that referenced this pull request Nov 21, 2024
ymotongpoo pushed a commit to ymotongpoo/opentelemetry.io that referenced this pull request Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants